feat: accept string-typed boolean task parameters - #1097

This new ValueError is reachable from a code path where nothing catches it, so an unexpected boolean wire form will kill the whole session rather than failing one action.

parameters_from_api_response is called in two places:

• JobDetails.from_boto (job_details.py:338), which is preceded by _validate_job_parameters — so a bad bool is rejected by the validator first, and that error is wrapped into a SessionActionError subclass by the caller. Fine.
• SessionActionQueue.dequeue for TASK_RUN parameters (scheduler/session_queue.py:535). Task parameters are never passed through _validate_job_parameters, and that call sits outside the try/except (ValueError, RuntimeError) block above it (session_queue.py:516-533) which converts step_details failures into StepDetailsError.

So a bare ValueError propagates out of dequeue(). Session._start_action (sessions/session.py:654-658) only catches SessionActionError, so it escapes to Session._run → Session.run, which sets _stop_fail_message, sets the stop event and re-raises — tearing down the entire session and cancelling all remaining queued actions, instead of reporting a single FAILED action with a useful message.

Before this change, value["bool"] for a task parameter was passed through verbatim and could not raise, so this is a new failure mode — and it is precisely the one the rollout makes likely: if the service ever emits a bool form other than exactly true/false (or native bool) for a task parameter, workers hard-fail sessions.

Suggest moving the parameters_from_api_response call at session_queue.py:535 inside a try/except (ValueError, RuntimeError) that raises StepDetailsError(action_id, SessionActionLogKind.TASK_RUN, str(e), step_id=step_id, task_id=task_id), matching how step_details resolution failures are already handled a few lines above. That keeps the strict validation but degrades to a graceful per-action failure.

Separate from the ValueError reachability point above: this list comprehension now iterates value["boolList"], where previously the value was passed through verbatim without being touched.

That introduces a TypeError (not ValueError) for a non-iterable boolList — e.g. {"boolList": None} raises TypeError: NoneType object is not iterable. For task-run parameters (scheduler/session_queue.py:535) there is no _validate_job_parameters pass and nothing catches it, so it escapes dequeue() past Session._start_actions except SessionActionError and tears down the whole session.

Worth calling out because a try/except (ValueError, RuntimeError) wrapper — the pattern used for the other job-entity errors in dequeue — would not catch this. If you wrap the call, include TypeError, or shape-check the value before iterating.

The new try closes the ValueError hole but leaves the adjacent TypeError hole open, so the session-teardown failure mode this block exists to prevent is still reachable.

parameters_from_api_response dispatches on "string" in value, "bool" in value, etc. (job_details.py:111-171) without first checking that value is a dict. Task parameters are never run through _validate_job_parameters — task_parameters_data comes straight from the unvalidated AssignedSession payload at line 534 — so if the service ever sends a parameter whose value is not a container (e.g. {"p": null} or {"p": 5}), the membership test raises TypeError: argument of type NoneType is not iterable, not ValueError. That escapes this except (ValueError, RuntimeError), propagates out of dequeue(), and since Session._start_action (sessions/session.py:653-658) only catches SessionActionError, it tears down the whole session and cancels every remaining queued action.

That is precisely the concern the PR itself raises one file over: the boolList shape-check comment at job_details.py:155-157 says it exists so callers "catching only ValueError" do not miss a TypeError. But only that one boolList instance was patched, while the dispatch chain above it has the same exposure for every parameter type.

Two cheap ways to close the class rather than the single instance:

• add TypeError here: except (ValueError, RuntimeError, TypeError) as e:, or
• shape-check at the top of the parameters_from_api_response loop, e.g. if not isinstance(value, dict): raise ValueError(f"Parameter {name} -- expected a dict but got {value!r}"), which also yields a message naming the offending parameter.

The second seems preferable since it fixes both call sites and keeps ValueError as the single decode-failure contract that the docstring and the new boolList check already assume.

This hand-enumerated vocabulary is now a hard gate: any spelling the service emits that is not in these two sets fails the action (StepDetailsError for TASK_RUN, ValueError out of validate_entity_data for job details). Before this change the value was passed through verbatim and could not fail. So the sets have to match what the producer actually emits, exactly — and the contents suggest they may not.

The set mixes two different rules. true/false/yes/no/on/off is a token list, but 1, 1.0, 0, 0.0 look like the tail of a numeric rule. If the spec rule is numeric, then 1.00, 01, +1, 0.0000 are all equally valid spellings of the same booleans, and the tests here deliberately assert those are rejected (bool-1.00-rejected, bool-01-rejected, bool-00-rejected). If the rule is a token list, it is not obvious why 1.0/0.0 are members at all. One of the two readings is wrong, and if it is the strict one, workers will fail live tasks on parameter values the service considers valid.

Two things worth doing:

1. Pin the accepted set against the actual producer rather than a prose reading of the spec — ideally openjd-models own boolean coercion if it exposes one (the repo already pins openjd-model >= 0.11.4, < 0.12, and this same decode feeds openjd.expr). Reusing it removes the divergence risk entirely and means the worker cannot drift when the spec adds a token. Re-deriving the vocabulary in the worker means every future spec addition is a worker-side hard-fail until a new agent ships — and agents in the field are not upgraded on the service is schedule.

2. If the set must stay local, consider whether an unrecognized token should really be terminal. Given rollout timing, a permissive path (log a warning and fall back to the pre-change behavior for unknown spellings) fails softer than rejecting, since the downside of a wrong coercion is one mis-evaluated expression while the downside of rejection is a failed customer task.

Also worth noting: _TRUE_STRINGS / _FALSE_STRINGS overlap with two existing vocabularies in this same package — telemetry.py:30 (_TRUE_VALUES = {"true", "yes", "on", "1"}) and the config_file.py settings docs (0, off, f, false, n, no, 1, on, t, true, y, yes). Three different accepted sets for the same concept in one codebase is a drift hazard even if each is individually correct for its own input.

High-level question on the approach: after this change bool/boolList become the only parameter types in this function that are not passed through verbatim, and the coercion is what forces the new hard-fail behaviour. It is worth confirming the coercion is actually required before accepting that risk.

Look at what the surrounding branches do with the other scalar types whose wire form is a string:

• int -> value["int"] passed through as a string (validator: isinstance(v, str))
• float -> passed through as a string
• chunkInt -> passed through as a string
• rangeExpr -> passed through as a string
• intList / floatList -> passed through as list[str] (validator: _is_str_list)

So the consumer downstream of ParameterValue already accepts un-parsed string forms for every other numeric type, including inside lists. If the service flip is normalizing bool to a string and boolList to list[str], that makes them consistent with int/intList rather than exceptional — which suggests the consumer may well accept them as-is too, and the docstring premise ("Open Job Description expression evaluation requires a native bool") may not hold, or may hold only for a path that native bool was already satisfying by accident before the flip.

This matters because the coercion is not free. It is what introduces the vocabulary gate, and therefore the new "unrecognized spelling fails the action" behaviour that did not exist before. If the string form is in fact accepted downstream, the lower-risk change is: widen the type annotations (which this PR already does), leave the value alone, and widen the validator to accept both forms — no coercion, no new failure mode, no worker-side copy of the spec vocabulary to keep in sync.

Two concrete things that would settle it:

1. Point at where a native bool is required — openjd.models ParameterValue.value type, or the expression evaluator that consumes it. If ParameterValue.value is annotated str, then note that the pre-change code was passing a native bool into a str-typed field, and the coercion is preserving a type error rather than fixing one.
2. Check the _v1/Rust path specifically: _to_rust_parameter_values (sessions/runtime/rust.py:122-152) hands value.value straight to the pyo3 TaskParameterValue. If that binding accepts the string form for BOOL the way it does for INT, the coercion buys nothing on the path that is presumably the migration target.

If the native bool genuinely is required, this all stands as-is and the only open item is the vocabulary itself (separate comment). If it is not, dropping the coercion removes the entire class of rollout risk.